Skip to content

Emit mixin forwarders as ordinary, non-bridge methods again #21890

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented Nov 5, 2024

Forward port of scala/scala#8037

Reverts #6352 and #6141.

Fixes #19270.

@hamzaremmal hamzaremmal self-assigned this Feb 17, 2025
@sjrd sjrd force-pushed the emit-mixin-forwards-as-non-bridges-again branch 2 times, most recently from 0af3b78 to 9a896d5 Compare February 25, 2025 14:53
sjrd added 5 commits April 30, 2025 14:45
This reverts commit 6d0f9ca.

Forward port of most commits in
scala/scala#8037

Compensate some of the consequences by adding the `MixedIn` flag.
In addition to the use seen in the diff in `BCodeHelpers`, the JS
backend has an existing test for `isOneOf(Bridge | MixedIn)` which
needs this compensation.
This is a forward port of
scala/scala@6fc2202
In Scala 2 that commit was reverting a commit that had stopped
emitting Java generic signatures. In dotc we never had that logic
before, so this is new code.
@sjrd sjrd force-pushed the emit-mixin-forwards-as-non-bridges-again branch from 9a896d5 to 66f90c6 Compare April 30, 2025 12:46
@jchyb jchyb self-assigned this Jul 10, 2025
@jchyb
Copy link
Contributor

jchyb commented Jul 10, 2025

The only remaining community-build test failure seems to be in scala-parallel-collections in SerializationStabilityTest.scala. After checking the failure, it seems to be a more-or-less expected consequence of this PR. Different generated classifies generated here cause a different SerialVersionUID to be used (which happened even when I modified the PR to only change flags, keeping the types of the methods same as before). The only solution is to stabilize the UID and change the test - thankfully there is a precedent for it:
scala/scala-parallel-collections#220 (here a direct consequence of a related flag removal affecting things)
scala/scala-parallel-collections#223
scala/scala-parallel-collections#240

I’ve submitted a PR that fixes this, so we can then bring it into the dotty-staging fork: scala/scala-parallel-collections#303

About the Mima errors, there are two types here:

  • DirectMissingMethodProblem - those appear in all of the mixins of Ordered, comparing across the old and new versions, they exist in both with different flags (which is the expected change here) so I feel that this is overall expected and could be filtered out.
  • FinalMethodProblem - we get errors like synthetic method sizeHint$default$2()Int in class scala.collection.mutable.ArrayBuilder is declared final in other version which is baffling as comparing the classfiles reveals that it is final in neither version. What used to not be final and after this PR is, is the generated sizeHint method, which is not reported (and which looks like it should be final anyway). I feel like this might be a bug in Mima.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scala 3.3.1: Concrete method inside trait marked (incorrectly?) as ACC_BRIDGE and ACC_SYNTHETIC
3 participants